Skip to content

DEV-15456: Added validation methods for group and filter fields to prevent regex injection - #4703

Merged
gregrholden merged 13 commits into
qatfrom
fix/DEV-15456-sanitize-postgres-input
Jul 29, 2026
Merged

DEV-15456: Added validation methods for group and filter fields to prevent regex injection#4703
gregrholden merged 13 commits into
qatfrom
fix/DEV-15456-sanitize-postgres-input

Conversation

@gregrholden

Copy link
Copy Markdown
Contributor

Description:

DEV-15456: Sanitize Postgres input [f112]
Fixing a vulnerability that allowed unauthenticated attackers to inject arbitrary Django ORM lookup suffixes (particularly __regex/__iregex) into aggregate group parameters and filter field paths, enabling regex DoS attacks and schema enumeration.
AC: Implement model-based validation that validates field paths against Django model schemas.

Technical Details:

Changes:

  • Aggregate Group Validation (common/mixins.py):
    • Added _resolve_field_path() method to validate group parameter fields segment-by-segment.
    • Validates fields against model schema using Django's _meta.get_field() API.
    • Supports legitimate FK traversal (e.g., treasury_account__federal_account_id).
    • Allows safe transform suffixes (e.g., __fy) defined in _sql_function_transformations (line 78).
    • Raises InvalidParameterException for invalid field paths.
    • NOTE: Also needed to update return value definitions across the file, and reorder imports, to pass lint tests.
  • Filter Field Validation (common/api_request_utils.py):
    • Added _validate_filter_field() and _resolve_field_path() methods to FilterGenerator.
    • Validates POST filter field parameters before creating Q objects.
    • Validates GET query parameter keys before use in filters.
    • Added _get_allowed_lookups() that derives safe Django lookups from operators dictionary.
    • Explicitly blocks __regex and __iregex lookups to prevent ReDoS attacks.
    • Supports compound lookups (e.g., __len__gt for ArrayField length checks).
    • Handles special cases: field arrays for range_intersect and search operations.

Requirements for PR Merge:

  1. Unit & integration tests updated
  2. API documentation updated (examples listed below)
    1. API Contracts
    2. API UI
    3. Comments
  3. Data validation completed (examples listed below)
    1. Does this work well with the current frontend? Or is the frontend aware of a needed change?
    2. Is performance impacted in the changes (e.g., API, pipeline, downloads, etc.)?
    3. Is the expected data returned with the expected format?
  4. Appropriate Operations ticket(s) created
  5. Jira Ticket(s)
    1. DEV-0

Explain N/A in above checklist:

@gregrholden
gregrholden requested a review from aguest-kc July 23, 2026 17:03
@gregrholden gregrholden self-assigned this Jul 23, 2026
@gregrholden gregrholden added the ready for review [PR] ready to be reviewed label Jul 23, 2026

@sethstoudenmier sethstoudenmier left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall LGTM! Ideally, we will move away from the v1 endpoints and replace with v2, but this is a great middle ground for now.

Comment thread usaspending_api/common/api_request_utils.py Outdated
if all(seg in allowed_lookups for seg in remaining_segments):
# All remaining segments are valid lookups, validation complete.
return
raise InvalidParameterException(f"Invalid field: {field_path}")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now uses f"" strings throughout per suggestion.

@james-at-kc james-at-kc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. improved code by using f-strings as suggested in previous approval.

@gregrholden
gregrholden merged commit c8cb9ac into qat Jul 29, 2026
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready for review [PR] ready to be reviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants